This bug is for crash report bp-8cd1ffcf-1710-4e4e-b527-93b5e0190929.

Top 10 frames of crashing thread:

0 libxul.so mozilla::AudioConverter::AudioConverter dom/media/AudioConverter.cpp:27
1 libxul.so mozilla::AudioSink::NotifyAudioNeeded mfbt/UniquePtr.h:617
2 libxul.so mozilla::detail::RunnableMethodImpl<mozilla::detail::Listener<RefPtr<mozilla::AudioData> >*, void  xpcom/threads/nsThreadUtils.h:1176
3 libxul.so mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:199
4 libxul.so nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:256
5 libxul.so non-virtual thunk to nsThreadPool::Run xpcom/threads/nsThreadPool.cpp
6 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
7 libxul.so <name omitted> xpcom/threads/nsThreadUtils.cpp:486
8 libxul.so mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:333
9 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290

Not a new crash since reports go back to 67.0 beta. The raw crash reason is:

MOZ_DIAGNOSTIC_ASSERT(aOut.Channels() <= 2 || aIn.Channels() == aOut.Channels()) (Only down/upmixing to mono or stereo is supported at this stage)

Since this is a diagnostic assertion the volume should be limited to nightly and beta. The assertion was introduced in bug 1542097 so CC'ing :jyavenard and :bryce.

The assert suggests that we should never end up with output channels > 2 unless we passing though. Our failing of the assert does rather suggest that it's possible to arrive at such a config though.

Searchfox is down right now, NI myself to look more at this once it's back up.

Based on the crash stacks this is the where the converter is being created with invalid args. For this to happen we must be creating the converter with inputChannels != outputChannels and outputChannels > 2. I.e. data->mChannels != mOutputChannels && mOutputChannels > 2 at that site.

mOutputChannels is set here, and tracing further back, the origin appears to be in the MDSM from after we've read metadata and following getting the first frame. I figure the audio callback is what is pushing data into the audio queue that we're using here (the data var).

My educated guess would be that we have a mismatch between the number of channels we're expecting based on our metadata, and the number of channels in the data we're getting from the audio callback, and that in the cases where the callback returns > 2 channel audio, we run into the assert.

Paul, does that sound plausible to you?

Alex, do you have time to look at this?

Sure, I keep the NI to myself to check later today.

The number of channels in data is being read by the third-party decoder during initialization. For example, for the (multichannel) sample [1] the channel info is set during initialization [2] and it is used later during decoding to create the AudioData [3]. Can the number of channels in metadata (mOutputChannels) and this one be different numbers?

[1] https://www2.iis.fraunhofer.de/AAC/ChID-BLITS-EBU-Narration.mp4
[2] https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp#99
[3] https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/dom/media/platforms/ffmpeg/FFmpegAudioDecoder.cpp#223,246

I don't know off the top of my head. I would have to delve the code to get an idea, which I don't think I have time to do in the near future.

I expect it will require a certain multi channel output setup to repro, but it would be useful if we had media that was triggering this.

This test case reproduces the issue on Windows.

Is bug 1376714 a dupe?

The testcase.mp4 file reproduced the following crash on windows 10:
https://crash-stats.mozilla.org/report/index/56440815-6139-4c14-aa7c-bda150191210
My attached device is the built-in one, stereo. It does not crash on Linux, it fails with a decoder error.

(In reply to Tyson Smith [:tsmith] from comment #9)

Is bug 1376714 a dupe?

Yeah, it looks similar.

The attached file from comment 7 is crashing in [1] because the aIn.Channels() returns 2 and aOut.Channels() returns 3. AudioConverter supports upmix/downmix to mono/stereo.

[1] https://searchfox.org/mozilla-central/rev/690e903ef689a4eca335b96bd903580394864a1c/dom/media/AudioConverter.cpp#27-29

The stack is different though:

xul.dll!mozilla::AudioConverter::AudioConverter(const mozilla::AudioConfig & aIn, const mozilla::AudioConfig & aOut) Line 27
	at c:\Users\achro\repos\mozilla-central\dom\media\AudioConverter.cpp(27)
[Inline Frame] xul.dll!mozilla::MakeUnique(mozilla::AudioConfig && aArgs, mozilla::AudioConfig && aArgs) Line 617
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\mozilla\UniquePtr.h(617)
xul.dll!mozilla::AudioSink::NotifyAudioNeeded() Line 379
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\AudioSink.cpp(379)
xul.dll!mozilla::AudioSink::Init(const mozilla::MediaSink::PlaybackParams & aParams, RefPtr<mozilla::MozPromise<bool,nsresult,0>> & aEndedPromise) Line 85
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\AudioSink.cpp(85)
xul.dll!mozilla::AudioSinkWrapper::Start(const mozilla::media::TimeUnit & aStartTime, const mozilla::MediaInfo & aInfo) Line 170
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\AudioSinkWrapper.cpp(170)
xul.dll!mozilla::VideoSink::Start(const mozilla::media::TimeUnit & aStartTime, const mozilla::MediaInfo & aInfo) Line 283
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\VideoSink.cpp(283)
xul.dll!mozilla::MediaDecoderStateMachine::StartMediaSink() Line 3201
	at c:\Users\achro\repos\mozilla-central\dom\media\MediaDecoderStateMachine.cpp(3201)
xul.dll!mozilla::MediaDecoderStateMachine::MaybeStartPlayback() Line 2846
	at c:\Users\achro\repos\mozilla-central\dom\media\MediaDecoderStateMachine.cpp(2846)
xul.dll!mozilla::MediaDecoderStateMachine::DecodingState::Step() Line 2337
	at c:\Users\achro\repos\mozilla-central\dom\media\MediaDecoderStateMachine.cpp(2337)
[Inline Frame] xul.dll!mozilla::detail::RunnableMethodArguments<>::applyImpl(nsMemoryReporterManager * o, nsresult(nsMemoryReporterManager::*)() m, mozilla::Tuple<> & args, std::integer_sequence<unsigned long long>) Line 1124
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h(1124)
[Inline Frame] xul.dll!mozilla::detail::RunnableMethodArguments<>::apply(nsMemoryReporterManager * o, nsresult(nsMemoryReporterManager::*)() m) Line 1130
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h(1130)
xul.dll!mozilla::detail::RunnableMethodImpl<nsMemoryReporterManager *,nsresult (nsMemoryReporterManager::*)(),1,mozilla::RunnableKind::Standard>::Run() Line 1179
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h(1179)
xul.dll!mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() Line 200
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\mozilla\TaskDispatcher.h(200)
xul.dll!mozilla::TaskQueue::Runner::Run() Line 203
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\TaskQueue.cpp(203)
xul.dll!nsThreadPool::Run() Line 306
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThreadPool.cpp(306)
xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 1251
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThread.cpp(1251)
xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 486
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThreadUtils.cpp(486)
xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate) Line 333
	at c:\Users\achro\repos\mozilla-central\ipc\glue\MessagePump.cpp(333)
[Inline Frame] xul.dll!MessageLoop::RunInternal() Line 315
	at c:\Users\achro\repos\mozilla-central\ipc\chromium\src\base\message_loop.cc(315)
xul.dll!MessageLoop::RunHandler() Line 309
	at c:\Users\achro\repos\mozilla-central\ipc\chromium\src\base\message_loop.cc(309)
xul.dll!MessageLoop::Run() Line 291
	at c:\Users\achro\repos\mozilla-central\ipc\chromium\src\base\message_loop.cc(291)
xul.dll!nsThread::ThreadFunc(void * aArg) Line 460
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThread.cpp(460)
nss3.dll!_PR_NativeRunThread(void * arg) Line 421
	at c:\Users\achro\repos\mozilla-central\nsprpub\pr\src\threads\combined\pruthr.c(421)
nss3.dll!pr_root(void * arg) Line 140
	at c:\Users\achro\repos\mozilla-central\nsprpub\pr\src\md\windows\w95thred.c(140)
[External Code]
[Inline Frame] mozglue.dll!mozilla::interceptor::FuncHook<mozilla::interceptor::WindowsDllInterceptor<mozilla::interceptor::VMSharingPolicyShared<mozilla::interceptor::MMPolicyInProcess,1>>,void (*)(int, void *, void *)>::operator()(int &) Line 141
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsWindowsDllInterceptor.h(141)
mozglue.dll!patched_BaseThreadInitThunk(int aIsInitialThread, void * aStartAddress, void * aThreadParam) Line 565
	at c:\Users\achro\repos\mozilla-central\mozglue\dllservices\WindowsDllBlocklist.cpp(565)
[External Code]

The patch above avoids calling the converter if the conversion is not possible. This fixes the crash. However, I am not sure if we want to avoid the crash using that or prevent the error somewhere deeper in the decoder before getting here. I NI Bryce in case he knows better.

Hi Alex, anything we can do to get this bug un-stuck?

I'll ping Bryce in the chat to see what we can do about it.

Debugging this on Windows:

• https://searchfox.org/mozilla-central/rev/eef39502e08bcd3c40573c65a6827828dce4a032/dom/media/mediasink/AudioSink.cpp#361 gets data with mChannels = 3, then mChannels = 2. This prompts recreation of the converter, which then hits the assert we see here.
• https://searchfox.org/mozilla-central/rev/eef39502e08bcd3c40573c65a6827828dce4a032/dom/media/platforms/wmf/WMFAudioMFTManager.cpp#301 is where that number originates (for my Windows test case). It looks like that data is populated when we process output from the decoder at https://searchfox.org/mozilla-central/rev/eef39502e08bcd3c40573c65a6827828dce4a032/dom/media/platforms/wmf/WMFAudioMFTManager.cpp#204

I've looked at the metadata for the testcase.mp4 file and it suggests it should have 2 channels in the audio track. I imagine it's the actual audio data that the decoder is basing its channel count on, but haven't delved into that.

Seems to me the files triggering this are likely bogus, so it makes sense to just fail if we're getting configs the stop us creating a convertor.

Thank you Bryce for spending time on this. In this case would you like to review my patch?

Actually I have assigned you already. Feel free to redirect or drop the review. Thanks

Backed out changeset 8b56bdf1ba13 (bug 1646508) for gv-junit failures

Backout: https://hg.mozilla.org/integration/autoland/rev/c5b9cb03ddd836ae74961c1695899fdb6463cf69

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0952f7dec63d88752aa7d52ba8b0da098895316b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306809243&repo=autoland&lineNumber=1422

(In reply to Cristina Coroiu [:ccoroiu] from comment #24)

Backed out changeset 8b56bdf1ba13 (bug 1646508) for gv-junit failures

Backout: https://hg.mozilla.org/integration/autoland/rev/c5b9cb03ddd836ae74961c1695899fdb6463cf69

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0952f7dec63d88752aa7d52ba8b0da098895316b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306809243&repo=autoland&lineNumber=1422

Looks like this ended up on the wrong bug?

Yes, sorry about that and thank you for the heads-up!

https://hg.mozilla.org/mozilla-central/rev/0952f7dec63d

Is this something we should consider uplifting to ESR78?

Comment on attachment 9115183 [details]
Bug 1584959 - Avoid calling the converter if the conversion is not possible.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a high volume assert crash in Nightly that has been converted to a decoder error. The assert is not activated in ESR so the way we react when the error exists is undefined behavior.
• User impact if declined: Assert crash/Undefined behavior in corrupted audio files
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It has backed in Nightly with success. Easy fix.
• String or UUID changes made by this patch:

Comment on attachment 9115183 [details]
Bug 1584959 - Avoid calling the converter if the conversion is not possible.

Approved for 78.1esr.

Can we land the test from this bug too?

https://hg.mozilla.org/releases/mozilla-esr78/rev/6dc7053ecfe1

I don't think we have an automated test for this. Do you have Tyson?

I don't have anything other than the mp4 file I've already attached.

Oops I would like to land this patch with Bug 1652467, accidentally used the wrong bug number. I will dup 1652467 here.

https://hg.mozilla.org/mozilla-central/rev/96c82012d855